Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

pageserver: coalesce index uploads when possible #10248

Open
wants to merge 1 commit into
base: erik/upload-reorder
Choose a base branch
from

Conversation

erikgrinaker
Copy link
Contributor

@erikgrinaker erikgrinaker commented Dec 30, 2024

Problem

With upload queue reordering in #10218, we can easily get into a situation where multiple index uploads are queued back to back, which can't be parallelized. This will happen e.g. when multiple layer flushes enqueue layer/index/layer/index/... and the layers skip the queue and are uploaded in parallel.

These index uploads will incur serial S3 roundtrip latencies, and may block later operations.

Resolves #10096.
Requires #10218.

Summary of changes

When multiple back-to-back index uploads are ready to upload, only upload the most recent index and drop the rest.

Copy link

github-actions bot commented Dec 30, 2024

7227 tests run: 6873 passed, 0 failed, 354 skipped (full report)


Flaky tests (2)

Postgres 17

Postgres 15

Code coverage* (full report)

  • functions: 31.4% (8477 of 27011 functions)
  • lines: 48.2% (67430 of 139861 lines)

* collected from Rust tests only


The comment gets automatically updated with the latest test results
1568f90 at 2025-01-04T10:38:55.123Z :recycle:

@erikgrinaker erikgrinaker force-pushed the erik/upload-reorder branch 9 times, most recently from 91be562 to dad6139 Compare January 2, 2025 14:46
@erikgrinaker erikgrinaker force-pushed the erik/upload-index-coalesce branch from 06f1d7a to 76698d5 Compare January 2, 2025 15:07
@erikgrinaker erikgrinaker force-pushed the erik/upload-reorder branch 3 times, most recently from e207145 to 419019e Compare January 4, 2025 09:07
@erikgrinaker erikgrinaker force-pushed the erik/upload-index-coalesce branch from 76698d5 to 9d54d23 Compare January 4, 2025 09:20
@erikgrinaker erikgrinaker marked this pull request as ready for review January 4, 2025 09:21
@erikgrinaker erikgrinaker requested a review from a team as a code owner January 4, 2025 09:21
@erikgrinaker erikgrinaker force-pushed the erik/upload-index-coalesce branch from 9d54d23 to 1568f90 Compare January 4, 2025 09:37
Copy link
Contributor

@VladLazar VladLazar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Coalesced indices are always in the same generation, so this feels pretty safe.
Coalescing won't kick in until the flush loop stops inserting barriers though, right?

@erikgrinaker
Copy link
Contributor Author

Coalescing won't kick in until the flush loop stops inserting barriers though, right?

That's right.

@erikgrinaker
Copy link
Contributor Author

Coalesced indices are always in the same generation, so this feels pretty safe.

Seems worth making that explicit, I'll add a condition.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants